feat(workhub): resume delegated work from the coordination conversation - #4713

Reviewed at cbfb198. The problem is real (Slice 5's exit sentence names resume), and the core of the design holds: the resume goes through the Action Gate, the Gate revalidates the active set and the named Session on the Host, and planResume/resumeDelegation call the same turn.resume.query / turn.resume.start handlers the interrupted-turn banner uses. Deriving the continuation turnId from the delegation and source run is fine too; that id has always been caller-supplied, so this is a stronger caller, not a second authority. One P1 and four P2s before it can merge, plus an ordering constraint with #4699.

P1: after a resume, Stop Payments cannot stop the work and reports it terminated. The continuation Turn is admitted with content: null and no source message (root-turn-coordinator.ts:2873-2879), so readMessageExecutionDisposition keeps resolving the delegation to the original root turn/run. retireDelegation builds its identity from that disposition (execution-composition.ts:1503-1512) and stopOwnedWorkHubRoot compares it against the active registry (:2130), where the continuation lives under wht_…. Nothing is stopped; coordinator.read(original) is failed/cancelled (that is why the resume was allowed), so the outcome is already_terminal, the fact is committed, and the delegation leaves the active set while the continuation keeps running in the target Session. replace and readDelegationRetirement take the same path, so "observe" is wrong too. Fix at the owner: the Host resolves "latest run on this root Turn's lineage" (the continuation admission already carries sourceRunId/sourceTurnId), and both retireDelegation and planResume use it. That also deletes the next item.

P2: lineage is recovered by scanning the coordination transcript, which is exactly what #4699 removes. workhub-coordination-coordinator.ts:424-432 does readMessagesSnapshot(WORKHUB_COORDINATION_SESSION_ID) to find the previous delegation_resume_resolved. That (turnId, runId) is Host-owned state mirrored into the transcript. #4699 deletes readMessagesSnapshot from CoordinationStores and replaces the unbounded scan with a paged, incremental active-assignment projection, so this hunk will not typecheck after it lands; both branches also claim epoch 113. #4699 has to go first, and this PR should rebase onto it and take the Host-side lineage from the P1 fix instead of the scan.

P2: already_running is decided from the persisted run snapshot without asking the Host, and every failure becomes parked. execution-composition.ts:1425-1431 returns already_running for admitted/created/running, which is the state a Host crash leaves behind; the stop path has a recovering outcome for exactly this window and its comment explains why. Then :1418-1422, :1446-1452 and :1473-1476 collapse a bare catch, turn.resume.query park reasons and every !ok (host_draining, internal_failure, session_archived, …) into parked and commit it as an immutable delegation_resume_resolved. Forward the Host's park reason, let operation failures surface as errors rather than facts, and give resume a recovering exit.

P2: the Host execution side has no coverage. planResume / resumeDelegation (execution-composition.ts:1396-1483) are stubbed in the coordinator test (:1636-1648) and untouched by the gate test, so lineage selection, the already_running decision, the error-to-parked mapping and the P1 path are all outside the suite. The resume_work branches in protocol/workhub-coordination.ts:541-570, 681-691 are not in the protocol test either. The P1 scenario is the natural regression.

P2: continue / 继续 / restart / 重新开始 should not be resume verbs. Continue Payments and 继续支付任务 are how people hand the next instruction to a Session; with a Session of that name the text is consumed as a resume (workhub-controller.ts:505 returns before ordinary routing) and, on already_running, the user reads "no need to resume" and their message was never sent. Restart Payments maps to the same derived continuation Turn (workhub-coordination-action-gate.ts:1163), which is the opposite of what the word asks. The PR's own comment at workhub-creation-intent.ts:113-117 names the ambiguity; the name-match guard does not help when the whole message is the verb plus the name. Keep resume / 恢复 / 接着跑, drop the other four.

Smaller, in one pass with the above:

• workhub-controller.ts:505-556 is :562-616 with four tokens changed; route-policy already extracted resolveNamedDelegationAction for the same reason. One submitNamedDelegationAction helper takes both to ~6 lines. Same in core: directWorkHubResumeCue/Target copy the stop pair line for line, and reuse UNSAFE_STOP_TARGET / stripMatchingStopQuotes under stop's name.
• WorkHubRequestIntent.resume.cue has no reader that imperative does not already answer (workhub-route-policy.ts:190 vs :255).
• whr_ + sha256(actionId) is already the runId prefix for assign; here it names a message id.
• workhub-surface.tsx:586-588: the frame state reads stop?.outcome but not resume?.outcome, so a parked resume renders as a completed turn.
• The body is out of date on three facts: epoch is 112 → 113, migration 39 rebuilds workhub_action_claims (SQLite cannot widen a CHECK in place; the migration itself is written correctly), and the Host-side stop does not use workHubNamedDelegationActionTargetsSession. Please rewrite the compatibility section before squash.

This is a user-visible surface (three new outcome states, two clarification prompts, three locales) with no before/after evidence in the body. Manual checks that remain after the fixes: delegate → interrupt → Resume X → Stop X, confirm the continuation actually stops and the card reflects it; kill the Host mid-run and resume after restart, confirm it does not answer already_running; a parked resume's frame state.

Evidence boundary: static read at cbfb198 against c180a2bac3 plus the #4699 branch; git merge-tree for the conflict; no build, no tests, no Desktop launched. The P1 chain is derived from the admission shape and isActiveWorkHubRoot, not observed.

AI-assisted review: drafted with Maka; I verified the retirement identity path, the transcript scan and the epoch/migration facts myself.

Re-reviewed at 6c62c31, reading the PR's own delta over the #4699 commit it sits on (28 files, +2159/−157). Every item from the last round is closed in code:

• P1: readLatestRootTurnLineage (root-turn-coordinator.ts:523) walks safe_boundary_continuation admissions forward from the Host's own ledger, and readDelegationRetirement, retireDelegation and planResume all use it. The delegate → interrupt → Resume → Stop regression goes through the production composition and asserts the continuation is cancelled.
• Transcript scan: gone. Lineage comes from the Host; the two new coordination messages carry the plan and its outcome, and sourceRuntimeEventHighWater gives the persisted plan a real job on replay. Migration 39 rebuilds workhub_action_claims inside one BEGIN IMMEDIATE, no index or FK on that table, so it is correct.
• already_running now asks the Host; recovering throws operation_unavailable without writing a fact; park reasons pass through; !ok throws.
• planResume / resumeDelegation, the Gate and the resume_work protocol branches are under test through their owners.
• Verbs are resume / 恢复 / 接着跑 only, with the four rejected forms asserted negative.
• The duplicated controller blocks, cue, the whr_ reuse, resume?.outcome on the surface and the three stale body facts are all fixed.

One new P1, then three P2s, then ordering.

P1: in a default build, resume always fails and records the failure as a durable fact. safeBoundaryResumeEnabled is process.env.MAKA_RUNTIME_SAFE_BOUNDARY_RESUME === '1' (execution-composition.ts:1024), off unless set. With it off the kernel skips continuation inspection (runtime-kernel.ts:647) and turn.resume.query parks with resume_feature_disabled; planResume turns that into { kind: 'parked' } (execution-composition.ts:1432), and the coordinator commits delegation_resume_requested + delegation_resume_resolved for it, then the surface says "Could not carry this work on. Open its Session to see why". So on the shipped configuration the new command fails every time, sends the user to a Session that has nothing wrong with it, and leaves two immutable records per attempt. The one useful fact, the park reason, reaches WorkHubSubmission.parkReason (workhub-controller.ts:413) and nothing reads it. Treat resume_feature_disabled like recovering: no persisted fact, operation_unavailable, and a clarification that says the runtime has resume off (the CLI already has the wording at pi-tui-runner.ts:348). The body's Behavior section should state the flag.

P2: CI is red on formatting. Biome wants packages/core/src/session.ts:1729-1730 on one line; npm run format. The body says lint passed, which is a different check.

P2: resume and stop disagree on what "one delegation on this Session" means. #resumeSource (workhub-coordination-action-gate.ts:618-627) rejects whenever onTarget.length !== 1. #stopSource (:689-700) first drops delegations whose work is retired and accepts if one is still holding work. Delegate to Payments twice, let the first finish, interrupt the second: Stop Payments resolves, Resume Payments answers "this name matches several pieces of work". That is the rule the PR's own route-policy comment says must not happen, broken on the Host side. Reuse stop's retirement filter in one helper.

P2: lineage resolution reads the Session's whole root-admission ledger. readLatestRootTurnLineage calls listRootTurnAdmissionsForRecovery(sessionId), which materialises and chain-validates every root admission (up to 1 MB each) for the Session, and it now sits under readDelegationRetirement, which the stop path calls once per active delegation on the target. A long-lived Session pays an unbounded read on every stop, replace or resume; that is the shape #4699 just removed from the transcript, moved to a different ledger. Add a store query that returns only safe_boundary_continuation admissions for the Session (or walks forward from a given turnId) instead of the full chain.

Smaller:

• parkReason on WorkHubSubmission has no reader outside tests; either it lands on screen with the P1 fix or it goes, same as cue last time.
• isWorkHubCoordinationFailure replaces the deleted re-export with a new export; workhub-surface.tsx:30-38 already imports from the controller, so a direct value import of the class is one fewer concept. #4822's ask was three lines deleted, not one deleted and one added.
• In resolveNamedDelegationAction the unnamedReason branch is unreachable for resume (imperative is only true with a target), and the two kind === 'resume' ? … : … return blocks in submitNamedDelegationAction have identical structure.

Ordering with #4699: unchanged, #4699 first, then rebase; the head is on an older #4699 commit and a5e1832283 has landed on that branch since. git merge-tree reports five conflicts, four mechanical (epoch table in protocol/index.ts, adjacent hunks in execution-composition.ts, two test files). The one that needs care: a5e1832283 changes sessionActions.assign from returning committedAssignment to taking a publishCommittedAssignment callback, in the same lines where this PR extracts the CoordinationSessionActions type (workhub-coordination-coordinator.ts:139-156); redo the extraction on the new signature. Keep #resume's beforeAppend calling the internal #listActiveAssignments as stop does, not the #admission.run-wrapped effect, or it re-enters the lease.

Manual acceptance after the fixes (three new outcome strings, two clarifications, three locales, no screenshots in the body): default build with the flag unset, Resume X should clarify rather than record; flag on, delegate → interrupt → Resume X → Stop X and watch the card; kill the Host mid-run, restart, Resume X should be retryable, not already_running; the two-delegations case above; parked frame state, light and dark, before/after.

Evidence boundary: static read of 6c62c3147e against 29367f7b1c and a5e1832283, plus one read-only biome format check; no build, no suites, no Desktop.

AI-assisted review: drafted with Maka; I verified the flag path, the resume/stop asymmetry and the conflict set myself.

Reviewed current head b2289fba53905374c717758b84a513d28bf20a5d (OPEN). One P1 and one P3 below. This is a feature; merging is a human decision regardless. Scope note: schema migration and protocol boundaries were reviewed line by line; the WorkHub controller/route-policy/surface files (~10 files) were not individually reviewed — flagged below, not covered.

P1 — the new unique index can break opening existing databases

sqlite-core-execution-schema.ts bumps the version 7 → 8 with a unique partial index on (session_id, sourceTurnId, sourceRunId) for safe_boundary_continuation rows. But that kind is not new — it already exists on main (runtime-invocation.ts:263) — and today's admitRootTurn dedupes only on turn_id, never checking the source triple. So two continuation records from the same boundary with different turn_ids can legally coexist today — exactly what the index wants to forbid.

If any installed database already holds such a pair, CREATE UNIQUE INDEX throws inside a bare db.exec that runs at database open (operational-state-store.ts:460, operational-target-schema.ts:71) — the outcome is not "feature unavailable" but "database won't open". There is no dedup or backfill step, and the new test only exercises the happy path on a fresh empty database. Whether such duplicates exist in the wild depends on whether historical flows could produce two turns from one boundary — that is for the author/maintainers to confirm, and the severity hinges entirely on the answer: impossible → P3 (missing migration test); possible → release-blocking.

Suggested fix: dedupe first (keep earliest or latest), or build a non-unique index and enforce uniqueness on the write path; either way, add a migration test with pre-existing duplicates.

P3 — epoch 113 is claimed by two open PRs; merge order matters

Main is at epoch 112. This PR writes 113 ("WorkHub exposes current active delegation linkage") plus 114 for itself — i.e. it stacks on a not-yet-merged sibling. Meanwhile PR #3299 independently claims 112 → 113 for a different reason. Whoever merges first takes 113; the other must renumber, or the epoch comments (the only trail for future cross-version handshake debugging) will mismatch real history. This is a merge-ordering dependency for planning, not a defect of this PR.

Verified good (worth stating)

The session-metadata migration 39 is exemplary: the comment explains why the table must be rebuilt (SQLite cannot relax a CHECK in place); rename-then-rebuild loses nothing (the v38 table deliberately had no indexes or foreign keys); explicit column lists on INSERT and SELECT; runs in a transaction (BEGIN IMMEDIATE); and a version-vs-entries-count assertion catches forgotten bumps. Likewise, readRootTurnContinuationAdmission re-validates the three fields after the index lookup instead of trusting json_extract blindly.

What I could not judge

The app was not run; resume-flow interaction unobserved. Whether duplicate boundary records exist in the wild (see P1) needs author/maintainer confirmation.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Follow-up to my review above, same head b2289fba53905374c717758b84a513d28bf20a5d: this covers the controller/UI portion that the first review explicitly left out (supplemental review of workhub-coordination-port.ts, workhub-controller.ts, workhub-route-policy.ts, workhub-surface.tsx, preload and related copy). One P2 and two P3s; still no P0–P1 from either part. Feature — merging remains a human decision.

The biggest win here is deleted code: the renderer used to replay the whole coordination history (rebuildCompleteHistory: loop loadBefore until no older, then loadAround) on every reset — linear in conversation length — just to derive current state. Now it asks the host (deps.candidates()), which owns the authoritative data. Direction is right, which is exactly why the P2 below is worth fixing.

P2 — one Host IPC per transcript batch, with no coalescing

At the end of the batch callback, if (handle && (reset || changed || batch.ready)) scheduleRefresh(refreshGeneration) queues one deps.candidates() onto the serial refreshLane — per batch. Bursty transcript batches therefore produce one Host round-trip each. Please coalesce (e.g. trailing-edge or generation check) so a burst resolves to a single refresh.

P3 — resume with a verb but no clear object gets no feedback at all

Entry handling is asymmetric: stop (workhub-route-policy.ts) with a cue but no usable target falls into a clarification branch that asks the user back; resume with if (!action.imperative || !action.target) return { kind: 'not_requested' } silently treats "keep working on that thing" as never requested — the message lands as ordinary chat and the user likely believes an instruction was issued. Please clarify or execute.

P3 — resume_target_unavailable merges a transient failure with a permanent state

workhub-controller.ts:425-429 maps both operation_unavailable and host_not_ready to the same clarification. But host_not_ready is transient (host still starting; retry helps) while operation_unavailable is capability-based (this host never enabled safe-boundary resume; retry never helps). Please distinguish them so the user knows whether to wait or stop trying.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.